Re: How was the majordomo bug found ?
Evil Pete (shipley@merde.dis.org)
Fri, 10 Jun 1994 11:05:03 -0700
------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5296.771271502.1@merde.dis.org>
>>From the developper's point of view, using system() or even popen() is a
>single obvious line of C code, fork()/exec() combination needs about a dozen
>of lines...
>
>>From the patches from Brent Chapman, it seems that majordomo was using
>system() or popen()...
no offense Brent but any programmer worth a hill of beans will not use
system()/popen() especially on untrusted data (eg: look how taintperl
handles such things).
years ago when I was searching for security problems to document
I did a `find /usr/src -name *.c -print | xargs egrep "system|popen`
and found a ton-o-ways.
>
>There should indeed be a FAQ about how to write 'secure programs'.
>
I have a document I wrote that talks of this, I will see about digging
it out.
here is a code sample (from the doc) the I wrote to locate programs
from a path
------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5296.771271502.2@merde.dis.org>
Content-Description: findp.c
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/param.h>
/* find path to a file */
/* Peter M Shipley 1987 */
char *findp(name)
char *name;
{
char *path, *p;
struct stat statbuf;
static char buffy[MAXPATHLEN +1];
/* get path from environment */
if( (path = getenv("PATH")) == NULL)
path = "/bin:/usr/bin:/usr/ucb";
/* copy modifiable copy for strtok */
path = strcpy(malloc(strlen(path) +1), path);
/* parse the path */
for(p = strtok(path, ":"); p != NULL ; p = strtok((char *) NULL, ":")) {
#ifdef SECURE
/* path must be absolute, not writable by other, owned by root or bin */
if ( *p != '/'
|| (stat(p, &statbuf) == -1)
|| (statbuf.st_mode & S_IWOTH)
|| (statbuf.st_uid != 0 && statbuf.st_uid != 3))
continue;
#endif
/* assemble path */
(void) strcpy(buffy, (*p == NULL ? "." : p));
(void) strcat(buffy, "/");
(void) strcat(buffy, name);
/* test for files existance */
if (stat(buffy, &statbuf) == 0)
break;
} /* for each in path */
return( ( p ? buffy : (char *) NULL) );
}
------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5296.771271502.3@merde.dis.org>
While you are at it you might want to add the code:
for (cpp = environ; cp = *cpp; /* void */) {
char **xpp;
if(strncmp(cp, "LD_", 3) == 0) {
for (xpp = cpp; xpp[0] = xpp[1]; xpp++);
} else {
cpp++;
}
}
somewhere before the fork/exec/system/popen is called. or better yet
delete the entire environment and replace it with a new (static) one.
(note that Pyramid and Apollo systems have special data in there environment
and will have to be special cased so that they are preserved.)
------- =_aaaaaaaaaa0--